diff --git a/client/app/pages/queries/view.js b/client/app/pages/queries/view.js index 276feab73b..cfa4d68ec3 100644 --- a/client/app/pages/queries/view.js +++ b/client/app/pages/queries/view.js @@ -157,7 +157,7 @@ function QueryViewCtrl( $scope.canEdit = currentUser.canEdit($scope.query) || $scope.query.can_edit; $scope.canViewSource = currentUser.hasPermission('view_source'); - $scope.canExecuteQuery = () => currentUser.hasPermission('execute_query') && !$scope.dataSource.view_only; + $scope.canExecuteQuery = () => $scope.query.is_safe || (currentUser.hasPermission('execute_query') && !$scope.dataSource.view_only); $scope.canForkQuery = () => currentUser.hasPermission('edit_query') && !$scope.dataSource.view_only; diff --git a/redash/handlers/queries.py b/redash/handlers/queries.py index 0fc867ae8e..d7b92716c1 100644 --- a/redash/handlers/queries.py +++ b/redash/handlers/queries.py @@ -16,6 +16,7 @@ require_permission, view_only) from redash.utils import collect_parameters_from_request from redash.serializers import QuerySerializer +from redash.utils.parameterized_query import ParameterizedQuery # Ordering map for relationships @@ -411,8 +412,9 @@ def post(self, query_id): require_access(query.groups, self.current_user, not_view_only) parameter_values = collect_parameters_from_request(request.args) + parameterized_query = ParameterizedQuery(query.query_text) - return run_query(query.data_source, parameter_values, query.query_text, query.id) + return run_query(parameterized_query, parameter_values, query.data_source, query.id) class QueryTagsResource(BaseResource): diff --git a/redash/handlers/query_results.py b/redash/handlers/query_results.py index 5d29b7650f..8e9b88953d 100644 --- a/redash/handlers/query_results.py +++ b/redash/handlers/query_results.py @@ -11,7 +11,7 @@ from redash.tasks import QueryTask from redash.tasks.queries import enqueue_query from redash.utils import (collect_parameters_from_request, gen_query_hash, json_dumps, utcnow) -from redash.utils.parameterized_query import ParameterizedQuery, dropdown_values +from redash.utils.parameterized_query import ParameterizedQuery, InvalidParameterError, dropdown_values def error_response(message): @@ -64,7 +64,7 @@ def run_query_sync(data_source, parameter_values, query_text, max_age=0): return None -def run_query(data_source, parameter_values, query_text, query_id, max_age=0, parameter_schema=None): +def run_query(query, parameters, data_source, query_id, max_age=0): if data_source.paused: if data_source.pause_reason: message = '{} is paused ({}). Please try later.'.format(data_source.name, data_source.pause_reason) @@ -73,7 +73,10 @@ def run_query(data_source, parameter_values, query_text, query_id, max_age=0, pa return error_response(message) - query = ParameterizedQuery(query_text, parameter_schema).apply(parameter_values) + try: + query.apply(parameters) + except InvalidParameterError as e: + abort(400, message=e.message) if query.missing_params: return error_response(u'Missing parameter value for: {}'.format(u", ".join(query.missing_params))) @@ -86,7 +89,10 @@ def run_query(data_source, parameter_values, query_text, query_id, max_age=0, pa if query_result: return {'query_result': query_result.to_dict()} else: - job = enqueue_query(query.text, data_source, current_user.id, metadata={"Username": current_user.email, "Query ID": query_id}) + job = enqueue_query(query.text, data_source, current_user.id, metadata={ + "Username": current_user.email, + "Query ID": query_id + }) return {'job': job.to_dict()} @@ -116,6 +122,8 @@ def post(self): query_id = params.get('query_id', 'adhoc') parameters = params.get('parameters', collect_parameters_from_request(request.args)) + parameterized_query = ParameterizedQuery(query) + data_source = models.DataSource.get_by_id_and_org(params.get('data_source_id'), self.current_org) if not has_access(data_source.groups, self.current_user, not_view_only): @@ -129,7 +137,7 @@ def post(self): 'query_id': query_id, 'parameters': parameters }) - return run_query(data_source, parameters, query, query_id, max_age) + return run_query(parameterized_query, parameters, data_source, query_id, max_age) ONE_YEAR = 60 * 60 * 24 * 365.25 @@ -163,23 +171,6 @@ def options(self, query_id=None, query_result_id=None, filetype='json'): return make_response("", 200, headers) - def _fetch_rows(self, query_id): - query = models.Query.get_by_id_and_org(query_id, self.current_org) - require_access(query.data_source.groups, self.current_user, view_only) - - query_result = models.QueryResult.get_by_id_and_org(query.latest_query_data_id, self.current_org) - - return json_loads(query_result.data)["rows"] - - def _convert_queries_to_enums(self, definition): - if definition["type"] == "query": - definition["type"] = "enum" - - rows = self._fetch_rows(definition.pop("queryId")) - definition["enumOptions"] = [row.get('value', row.get(row.keys()[0])) for row in rows if row] - - return definition - @require_permission('execute_query') def post(self, query_id): """ @@ -201,12 +192,13 @@ def post(self, query_id): max_age = int(max_age) query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org) - parameter_schema = query.options.get("parameters", []) - if not has_access(query.data_source.groups, self.current_user, not_view_only): - return {'job': {'status': 4, 'error': 'You do not have permission to run queries with this data source.'}}, 403 + allow_executing_with_view_only_permissions = query.parameterized.is_safe + + if has_access(query.data_source.groups, self.current_user, allow_executing_with_view_only_permissions): + return run_query(query.parameterized, parameters, query.data_source, query_id, max_age) else: - return run_query(query.data_source, parameters, query.query_text, query_id, max_age, parameter_schema=parameter_schema) + return {'job': {'status': 4, 'error': 'You do not have permission to run queries with this data source.'}}, 403 @require_permission('view_query') def get(self, query_id=None, query_result_id=None, filetype='json'): diff --git a/redash/models/__init__.py b/redash/models/__init__.py index 19cc8be623..bb012d4dbd 100644 --- a/redash/models/__init__.py +++ b/redash/models/__init__.py @@ -28,6 +28,7 @@ get_query_runner) from redash.utils import generate_token, json_dumps, json_loads from redash.utils.configuration import ConfigurationContainer +from redash.utils.parameterized_query import ParameterizedQuery from .base import db, gfk_type, Column, GFKBase, SearchBaseQuery from .changes import ChangeTrackingMixin, Change # noqa @@ -667,6 +668,10 @@ def lowercase_name(cls): "The SQLAlchemy expression for the property above." return func.lower(cls.name) + @property + def parameterized(self): + return ParameterizedQuery(self.query_text, self.options.get("parameters", [])) + @listens_for(Query.query_text, 'set') def gen_query_hash(target, val, oldval, initiator): diff --git a/redash/serializers.py b/redash/serializers.py index d809a1f73e..4306461dee 100644 --- a/redash/serializers.py +++ b/redash/serializers.py @@ -10,6 +10,7 @@ from redash import models from redash.permissions import has_access, view_only from redash.utils import json_loads +from redash.utils.parameterized_query import ParameterizedQuery def public_widget(widget): @@ -100,6 +101,7 @@ def serialize_query(query, with_stats=False, with_visualizations=False, with_use 'options': query.options, 'version': query.version, 'tags': query.tags or [], + 'is_safe': query.parameterized.is_safe, } if with_user: diff --git a/redash/utils/parameterized_query.py b/redash/utils/parameterized_query.py index f71442ee9d..1c489ce456 100644 --- a/redash/utils/parameterized_query.py +++ b/redash/utils/parameterized_query.py @@ -1,9 +1,7 @@ import pystache from functools import partial from flask_login import current_user -from redash.authentication.org_resolving import current_org from numbers import Number -from redash import models from redash.utils import mustache_render, json_loads from redash.permissions import require_access, view_only from funcy import distinct @@ -19,6 +17,9 @@ def _pluck_name_and_value(default_column, row): def _load_result(query_id): + from redash.authentication.org_resolving import current_org + from redash import models + query = models.Query.get_by_id_and_org(query_id, current_org) require_access(query.data_source.groups, current_user, view_only) query_result = models.QueryResult.get_by_id_and_org(query.latest_query_data_id, current_org) @@ -121,6 +122,11 @@ def _valid(self, name, value): return validate(value) + @property + def is_safe(self): + text_parameters = filter(lambda p: p["type"] == "text", self.schema) + return not any(text_parameters) + @property def missing_params(self): query_parameters = set(_collect_query_parameters(self.template)) diff --git a/tests/handlers/test_query_results.py b/tests/handlers/test_query_results.py index 2a06ae158c..455cb81704 100644 --- a/tests/handlers/test_query_results.py +++ b/tests/handlers/test_query_results.py @@ -136,6 +136,20 @@ def test_execute_new_query(self): self.assertEquals(rv.status_code, 200) self.assertIn('job', rv.json) + def test_prevents_execution_of_unsafe_queries_on_view_only_data_sources(self): + ds = self.factory.create_data_source(group=self.factory.org.default_group, view_only=True) + query = self.factory.create_query(data_source=ds, options={"parameters": [{"name": "foo", "type": "text"}]}) + + rv = self.make_request('post', '/api/queries/{}/results'.format(query.id), data={"parameters": {}}) + self.assertEquals(rv.status_code, 403) + + def test_allows_execution_of_safe_queries_on_view_only_data_sources(self): + ds = self.factory.create_data_source(group=self.factory.org.default_group, view_only=True) + query = self.factory.create_query(data_source=ds, options={"parameters": [{"name": "foo", "type": "number"}]}) + + rv = self.make_request('post', '/api/queries/{}/results'.format(query.id), data={"parameters": {}}) + self.assertEquals(rv.status_code, 200) + def test_access_with_query_api_key(self): ds = self.factory.create_data_source(group=self.factory.org.default_group, view_only=False) query = self.factory.create_query() diff --git a/tests/utils/test_parameterized_query.py b/tests/utils/test_parameterized_query.py index 8702d9b157..24e2016b4e 100644 --- a/tests/utils/test_parameterized_query.py +++ b/tests/utils/test_parameterized_query.py @@ -166,6 +166,24 @@ def test_raises_on_unexpected_param_types(self): with pytest.raises(InvalidParameterError): query.apply({"bar": "baz"}) + def test_is_not_safe_if_expecting_text_parameter(self): + schema = [{"name": "bar", "type": "text"}] + query = ParameterizedQuery("foo", schema) + + self.assertFalse(query.is_safe) + + def test_is_safe_if_not_expecting_text_parameter(self): + schema = [{"name": "bar", "type": "number"}] + query = ParameterizedQuery("foo", schema) + + self.assertTrue(query.is_safe) + + def test_is_safe_if_not_expecting_any_parameters(self): + schema = [] + query = ParameterizedQuery("foo", schema) + + self.assertTrue(query.is_safe) + @patch('redash.utils.parameterized_query._load_result', return_value={ "columns": [{"name": "id"}, {"name": "Name"}, {"name": "Value"}], "rows": [{"id": 5, "Name": "John", "Value": "John Doe"}]})