diff --git a/awx/api/filters.py b/awx/api/filters.py index 9f34723d48..7e55b312fa 100644 --- a/awx/api/filters.py +++ b/awx/api/filters.py @@ -89,7 +89,8 @@ class FieldLookupBackend(BaseFilterBackend): # those lookups combined with request.user.get_queryset(Model) to make # sure user cannot query using objects he could not view. new_parts = [] - for n, name in enumerate(parts[:-1]): + + for name in parts[:-1]: # HACK: Make project and inventory source filtering by old field names work for backwards compatibility. if model._meta.object_name in ('Project', 'InventorySource'): name = { @@ -111,6 +112,10 @@ class FieldLookupBackend(BaseFilterBackend): field = model._meta.pk else: field = model._meta.get_field_by_name(name)[0] + if isinstance(field, ForeignObjectRel) and getattr(field.field, '__prevent_search__', False): + raise PermissionDenied('Filtering on %s is not allowed.' % name) + elif getattr(field, '__prevent_search__', False): + raise PermissionDenied('Filtering on %s is not allowed.' % name) model = getattr(field, 'related_model', None) or field.model if parts: diff --git a/awx/api/generics.py b/awx/api/generics.py index e3bcdc221c..1c8a38a54c 100644 --- a/awx/api/generics.py +++ b/awx/api/generics.py @@ -26,6 +26,7 @@ from rest_framework import status from rest_framework import views # AWX +from awx.api.filters import FieldLookupBackend from awx.main.models import * # noqa from awx.main.utils import * # noqa from awx.api.serializers import ResourceAccessListElementSerializer @@ -297,7 +298,16 @@ class ListAPIView(generics.ListAPIView, GenericAPIView): if relationship.related_model._meta.app_label != 'main': continue fields.append('{}__search'.format(relationship.name)) - return fields + + allowed_fields = [] + for field in fields: + try: + FieldLookupBackend().get_field_from_lookup(self.model, field) + except PermissionDenied: + pass + else: + allowed_fields.append(field) + return allowed_fields class ListCreateAPIView(ListAPIView, generics.ListCreateAPIView): diff --git a/awx/conf/models.py b/awx/conf/models.py index bc6cfb3dfc..5c26e17c54 100644 --- a/awx/conf/models.py +++ b/awx/conf/models.py @@ -8,7 +8,7 @@ import json from django.db import models # Tower -from awx.main.models.base import CreatedModifiedModel +from awx.main.models.base import CreatedModifiedModel, prevent_search from awx.main.fields import JSONField from awx.main.utils import encrypt_field from awx.conf import settings_registry @@ -24,14 +24,14 @@ class Setting(CreatedModifiedModel): value = JSONField( null=True, ) - user = models.ForeignKey( + user = prevent_search(models.ForeignKey( 'auth.User', related_name='settings', default=None, null=True, editable=False, on_delete=models.CASCADE, - ) + )) def __unicode__(self): try: diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index d7b65d8107..2f4d02a7b9 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -42,7 +42,7 @@ _PythonSerializer.handle_m2m_field = _new_handle_m2m_field # Add custom methods to User model for permissions checks. -from django.contrib.auth.models import User # noqa +from django.contrib.auth.models import User # noqa from awx.main.access import * # noqa @@ -128,3 +128,6 @@ activity_stream_registrar.connect(User) activity_stream_registrar.connect(WorkflowJobTemplate) activity_stream_registrar.connect(WorkflowJobTemplateNode) activity_stream_registrar.connect(WorkflowJob) + +# prevent API filtering on certain Django-supplied sensitive fields +prevent_search(User._meta.get_field('password')) diff --git a/awx/main/models/ad_hoc_commands.py b/awx/main/models/ad_hoc_commands.py index 057924eda7..3636aa8e0a 100644 --- a/awx/main/models/ad_hoc_commands.py +++ b/awx/main/models/ad_hoc_commands.py @@ -83,10 +83,10 @@ class AdHocCommand(UnifiedJob, JobNotificationMixin): editable=False, through='AdHocCommandEvent', ) - extra_vars = models.TextField( + extra_vars = prevent_search(models.TextField( blank=True, default='', - ) + )) extra_vars_dict = VarsDictProperty('extra_vars', True) diff --git a/awx/main/models/base.py b/awx/main/models/base.py index bdee496bad..81e00f92c6 100644 --- a/awx/main/models/base.py +++ b/awx/main/models/base.py @@ -23,7 +23,7 @@ from crum import get_current_user # Ansible Tower from awx.main.utils import encrypt_field -__all__ = ['VarsDictProperty', 'BaseModel', 'CreatedModifiedModel', +__all__ = ['prevent_search', 'VarsDictProperty', 'BaseModel', 'CreatedModifiedModel', 'PasswordFieldsModel', 'PrimordialModel', 'CommonModel', 'CommonModelNameNotUnique', 'NotificationFieldsModel', 'PERM_INVENTORY_ADMIN', 'PERM_INVENTORY_READ', @@ -343,3 +343,21 @@ class NotificationFieldsModel(BaseModel): blank=True, related_name='%(class)s_notification_templates_for_any' ) + + + +def prevent_search(relation): + """ + Used to mark a model field or relation as "restricted from filtering" + e.g., + + class AuthToken(BaseModel): + user = prevent_search(models.ForeignKey(...)) + sensitive_data = prevent_search(models.CharField(...)) + + The flag set by this function is used by + `awx.api.filters.FieldLookupBackend` to blacklist fields and relations that + should not be searchable/filterable via search query params + """ + setattr(relation, '__prevent_search__', True) + return relation diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index b01d44802c..020121120b 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -1284,11 +1284,11 @@ class CustomInventoryScript(CommonModelNameNotUnique, ResourceMixin): unique_together = [('name', 'organization')] ordering = ('name',) - script = models.TextField( + script = prevent_search(models.TextField( blank=True, default='', help_text=_('Inventory script contents'), - ) + )) organization = models.ForeignKey( 'Organization', related_name='custom_inventory_scripts', diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 19a853a45f..00a68c69ca 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -117,10 +117,10 @@ class JobOptions(BaseModel): blank=True, default=0, ) - extra_vars = models.TextField( + extra_vars = prevent_search(models.TextField( blank=True, default='', - ) + )) job_tags = models.CharField( max_length=1024, blank=True, @@ -1252,10 +1252,10 @@ class SystemJob(UnifiedJob, SystemJobOptions, JobNotificationMixin): on_delete=models.SET_NULL, ) - extra_vars = models.TextField( + extra_vars = prevent_search(models.TextField( blank=True, default='', - ) + )) extra_vars_dict = VarsDictProperty('extra_vars', True) diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index c755de9f0a..3ae26eaf71 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -7,6 +7,7 @@ from django.contrib.contenttypes.models import ContentType from django.contrib.auth.models import User # noqa # AWX +from awx.main.models.base import prevent_search from awx.main.models.rbac import ( Role, RoleAncestorEntry, get_roles_on_resource ) @@ -86,10 +87,10 @@ class SurveyJobTemplateMixin(models.Model): survey_enabled = models.BooleanField( default=False, ) - survey_spec = JSONField( + survey_spec = prevent_search(JSONField( blank=True, default={}, - ) + )) def survey_password_variables(self): vars = [] @@ -215,11 +216,11 @@ class SurveyJobMixin(models.Model): class Meta: abstract = True - survey_passwords = JSONField( + survey_passwords = prevent_search(JSONField( blank=True, default={}, editable=False, - ) + )) def display_extra_vars(self): ''' diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index c2fe3b1c4f..99023c86e1 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -220,12 +220,13 @@ class AuthToken(BaseModel): app_label = 'main' key = models.CharField(max_length=40, primary_key=True) - user = models.ForeignKey('auth.User', related_name='auth_tokens', - on_delete=models.CASCADE) + user = prevent_search(models.ForeignKey('auth.User', + related_name='auth_tokens', on_delete=models.CASCADE)) created = models.DateTimeField(auto_now_add=True) modified = models.DateTimeField(auto_now=True) expires = models.DateTimeField(default=tz_now) - request_hash = models.CharField(max_length=40, blank=True, default='') + request_hash = prevent_search(models.CharField(max_length=40, blank=True, + default='')) reason = models.CharField( max_length=1024, blank=True, diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index a23091962f..f17b2b4c55 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -503,33 +503,33 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique editable=False, help_text=_("Elapsed time in seconds that the job ran."), ) - job_args = models.TextField( + job_args = prevent_search(models.TextField( blank=True, default='', editable=False, - ) + )) job_cwd = models.CharField( max_length=1024, blank=True, default='', editable=False, ) - job_env = JSONField( + job_env = prevent_search(JSONField( blank=True, default={}, editable=False, - ) + )) job_explanation = models.TextField( blank=True, default='', editable=False, help_text=_("A status field to indicate the state of the job if it wasn't able to run and capture stdout"), ) - start_args = models.TextField( + start_args = prevent_search(models.TextField( blank=True, default='', editable=False, - ) + )) result_stdout_text = models.TextField( blank=True, default='', diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index a6a1ccfe81..874822e013 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -11,7 +11,7 @@ from django.core.urlresolvers import reverse #from django import settings as tower_settings # AWX -from awx.main.models import UnifiedJobTemplate, UnifiedJob +from awx.main.models import prevent_search, UnifiedJobTemplate, UnifiedJob from awx.main.models.notifications import ( NotificationTemplate, JobNotificationMixin @@ -280,10 +280,10 @@ class WorkflowJobOptions(BaseModel): class Meta: abstract = True - extra_vars = models.TextField( + extra_vars = prevent_search(models.TextField( blank=True, default='', - ) + )) extra_vars_dict = VarsDictProperty('extra_vars', True) diff --git a/awx/main/tests/unit/api/test_filters.py b/awx/main/tests/unit/api/test_filters.py index fc707434c5..6570ada6f7 100644 --- a/awx/main/tests/unit/api/test_filters.py +++ b/awx/main/tests/unit/api/test_filters.py @@ -2,7 +2,11 @@ import pytest from rest_framework.exceptions import PermissionDenied from awx.api.filters import FieldLookupBackend -from awx.main.models import Credential, JobTemplate +from awx.main.models import (AdHocCommand, AuthToken, CustomInventoryScript, + Credential, Job, JobTemplate, SystemJob, + UnifiedJob, User, WorkflowJob, + WorkflowJobTemplate, WorkflowJobOptions) +from awx.main.models.jobs import JobOptions @pytest.mark.parametrize(u"empty_value", [u'', '']) @@ -38,3 +42,28 @@ def test_filter_on_related_password_field(password_field, lookup_suffix): with pytest.raises(PermissionDenied) as excinfo: field, new_lookup = field_lookup.get_field_from_lookup(JobTemplate, lookup) assert 'not allowed' in str(excinfo.value) + + +@pytest.mark.parametrize('model, query', [ + (AuthToken, 'request_hash__icontains'), + (User, 'password__icontains'), + (User, 'auth_tokens__key__icontains'), + (User, 'settings__value__icontains'), + (UnifiedJob, 'job_args__icontains'), + (UnifiedJob, 'job_env__icontains'), + (UnifiedJob, 'start_args__icontains'), + (AdHocCommand, 'extra_vars__icontains'), + (JobOptions, 'extra_vars__icontains'), + (SystemJob, 'extra_vars__icontains'), + (WorkflowJobOptions, 'extra_vars__icontains'), + (Job, 'survey_passwords__icontains'), + (WorkflowJob, 'survey_passwords__icontains'), + (JobTemplate, 'survey_spec__icontains'), + (WorkflowJobTemplate, 'survey_spec__icontains'), + (CustomInventoryScript, 'script__icontains') +]) +def test_filter_sensitive_fields_and_relations(model, query): + field_lookup = FieldLookupBackend() + with pytest.raises(PermissionDenied) as excinfo: + field, new_lookup = field_lookup.get_field_from_lookup(model, query) + assert 'not allowed' in str(excinfo.value)