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

Server-side parameter validation #3315

Merged
merged 61 commits into from
Jan 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
4a557c7
stop testing `collect_query_parameters`, it's an implementation detail
Dec 27, 2018
856e61c
add tests for `missing_query_params`
Dec 27, 2018
5bd751f
rename SQLQuery -> ParameterizedSqlQuery
Dec 30, 2018
f43c5f7
rename sql_query.py to parameterized_query.py
Dec 30, 2018
b608c5b
split to parameterized queries and parameterized SQL queries, where
Dec 30, 2018
ced3239
Merge branch 'tests-for-find-missing-params' into run-tree-validation…
Dec 30, 2018
8798475
Merge branch 'master' into run-tree-validations-only-on-sql-dialects
Dec 31, 2018
e49884d
move missing parameter detection to ParameterizedQuery
Dec 31, 2018
07df144
get rid of some old code
Jan 1, 2019
36b3045
fix tests
Jan 1, 2019
a72f781
Merge branch 'master' into run-tree-validations-only-on-sql-dialects
Jan 1, 2019
360f85c
set syntax to `custom`
Jan 1, 2019
5239eba
Merge branch 'master' into run-tree-validations-only-on-sql-dialects
Jan 1, 2019
a94f5d9
Merge branch 'run-tree-validations-only-on-sql-dialects' of github.co…
Jan 1, 2019
b0b7164
Merge branch 'master' into run-tree-validations-only-on-sql-dialects
Jan 3, 2019
23af64e
Merge branch 'master' into run-tree-validations-only-on-sql-dialects
Jan 6, 2019
a136cd0
Merge branch 'master' into run-tree-validations-only-on-sql-dialects
Jan 13, 2019
639c76f
revert the max-age-related refactoring
Jan 16, 2019
c18675a
Merge branch 'master' into run-tree-validations-only-on-sql-dialects
Jan 16, 2019
6abcedf
👋 tree validations 😢
Jan 16, 2019
5a12c23
BaseQueryRunner is no longer a factory for ParameterizedQuery, for now
Jan 16, 2019
bf41cc9
Merge branch 'master' into validate-parameters
Jan 17, 2019
71e249c
Merge branch 'master' into validate-parameters
Jan 20, 2019
8b7c9c4
add an endpoint for running a query by its id and (optional) parameters
Jan 20, 2019
866d4e2
Merge branch 'master' into textless-query-result-endpoint
Jan 20, 2019
27cd9af
Merge branch 'master' into textless-query-result-endpoint
Jan 20, 2019
d0d0fc4
Merge branch 'textless-query-result-endpoint' into validate-parameters
Jan 20, 2019
61ae426
adds parameter schema to ParameterizedQuery
Jan 20, 2019
6d74494
adds parameter schema validation (currently for strings)
Jan 20, 2019
07fb716
validate number parameters
Jan 20, 2019
80b06e9
validate date parameters
Jan 20, 2019
a685de9
validate parameters on POST /api/queries/<id>/results
Jan 20, 2019
f801b58
validate enum parameters
Jan 21, 2019
42e28e2
validate date range parameters
Jan 21, 2019
5829497
validate query-based dropdowns by preprocessing them at the handler
Jan 21, 2019
f9d1eb5
change _is_date_range to be a tad more succinct
Jan 21, 2019
4eb3935
a single assignment with a `map` is sufficiently explanatory
Jan 21, 2019
1e31775
Update redash/utils/parameterized_query.py
jezdez Jan 21, 2019
6c0387b
Update redash/utils/parameterized_query.py
jezdez Jan 21, 2019
fb81066
Update redash/utils/parameterized_query.py
jezdez Jan 21, 2019
f3cb6eb
Update redash/utils/parameterized_query.py
jezdez Jan 21, 2019
c23add1
Update redash/handlers/query_results.py
jezdez Jan 21, 2019
271cab6
Update redash/utils/parameterized_query.py
jezdez Jan 21, 2019
f7bfde9
build error message inside the error
Jan 21, 2019
3785f09
support all types of numbers as number parameters
Jan 21, 2019
8d8d1f8
check for permissions when populating query-based dropdowns
Jan 21, 2019
4625d31
Merge branch 'master' into textless-query-result-endpoint
Jan 23, 2019
3a8c18b
check for access to query before running it
Jan 23, 2019
ab60338
Merge branch 'master' into validate-parameters
Jan 23, 2019
d737c56
check for empty rows when populating query-based enums
Jan 23, 2019
944f6c1
don't bother loading query results if user doesn't have access
Jan 23, 2019
5e719d4
💥 on unexpected parameter types
Jan 23, 2019
855be00
parameter schema default is a list, not a dictionary
Jan 23, 2019
074c872
Merge branch 'textless-query-result-endpoint' into validate-parameters
Jan 23, 2019
7ba9f19
Merge branch 'master' into validate-parameters
Jan 23, 2019
2d841ea
Merge branch 'validate-parameters' of github.com:getredash/redash int…
Jan 23, 2019
37d0fe4
remove redundant null guards
Jan 24, 2019
e261149
Merge branch 'master' into validate-parameters
Jan 27, 2019
11fc681
Merge branch 'master' into validate-parameters
Jan 28, 2019
8509b6b
Merge branch 'master' into validate-parameters
Jan 28, 2019
87f1dc4
Merge branch 'master' into validate-parameters
Jan 29, 2019
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
27 changes: 23 additions & 4 deletions redash/handlers/query_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
require_permission, view_only)
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 import (collect_parameters_from_request, gen_query_hash, json_dumps, json_loads, utcnow)
from redash.utils.parameterized_query import ParameterizedQuery


Expand Down Expand Up @@ -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):
def run_query(data_source, parameter_values, query_text, query_id, max_age=0, parameter_schema=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You removed the handling of the None case here, because I assume that the callers will always pass a parameter_schema? If that's the case, then should probably remove the default value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually intentional. Handling of None is done in ParameterizedQuery.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that's the case, but didn't find it. Apparently I was looking in the wrong place. 🤦🏻‍♂️

if data_source.paused:
if data_source.pause_reason:
message = '{} is paused ({}). Please try later.'.format(data_source.name, data_source.pause_reason)
Expand All @@ -73,7 +73,7 @@ def run_query(data_source, parameter_values, query_text, query_id, max_age=0):

return error_response(message)

query = ParameterizedQuery(query_text).apply(parameter_values)
query = ParameterizedQuery(query_text, parameter_schema).apply(parameter_values)

if query.missing_params:
return error_response(u'Missing parameter value for: {}'.format(u", ".join(query.missing_params)))
Expand Down Expand Up @@ -154,6 +154,23 @@ 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)
rauchy marked this conversation as resolved.
Show resolved Hide resolved

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):
"""
Expand All @@ -171,11 +188,13 @@ def post(self, query_id):
max_age = int(params.get('max_age', 0))

query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
parameter_schema = map(self._convert_queries_to_enums,
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
else:
return run_query(query.data_source, parameters, query.query_text, query_id, max_age)
return run_query(query.data_source, parameters, query.query_text, query_id, max_age, parameter_schema=parameter_schema)

@require_permission('view_query')
def get(self, query_id=None, query_result_id=None, filetype='json'):
Expand Down
58 changes: 54 additions & 4 deletions redash/utils/parameterized_query.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import pystache
from numbers import Number
from redash.utils import mustache_render
from funcy import distinct

from dateutil.parser import parse

def _collect_key_names(nodes):
keys = []
Expand Down Expand Up @@ -33,17 +34,60 @@ def _parameter_names(parameter_values):
return names


def _is_date(string):
try:
parse(string)
arikfr marked this conversation as resolved.
Show resolved Hide resolved
return True
except ValueError:
return False


def _is_date_range(obj):
try:
return _is_date(obj["start"]) and _is_date(obj["end"])
except (KeyError, TypeError):
return False


class ParameterizedQuery(object):
def __init__(self, template):
def __init__(self, template, schema=None):
self.schema = schema or []
self.template = template
self.query = template
self.parameters = {}

def apply(self, parameters):
self.parameters.update(parameters)
self.query = mustache_render(self.template, self.parameters)
invalid_parameter_names = [key for (key, value) in parameters.iteritems() if not self._valid(key, value)]
if invalid_parameter_names:
raise InvalidParameterError(invalid_parameter_names)
else:
self.parameters.update(parameters)
self.query = mustache_render(self.template, self.parameters)

return self

def _valid(self, name, value):
definition = next((definition for definition in self.schema if definition["name"] == name), None)

if not definition:
return True

validators = {
"text": lambda value: isinstance(value, basestring),
"number": lambda value: isinstance(value, Number),
"enum": lambda value: value in definition["enumOptions"],
"date": _is_date,
"datetime-local": _is_date,
"datetime-with-seconds": _is_date,
"date-range": _is_date_range,
"datetime-range": _is_date_range,
"datetime-range-with-seconds": _is_date_range,
}

validate = validators.get(definition["type"], lambda x: False)

return validate(value)

@property
def missing_params(self):
query_parameters = set(_collect_query_parameters(self.template))
Expand All @@ -52,3 +96,9 @@ def missing_params(self):
@property
def text(self):
return self.query


class InvalidParameterError(Exception):
def __init__(self, parameters):
message = u"The following parameter values are incompatible with their type definitions: {}".format(", ".join(parameters))
super(InvalidParameterError, self).__init__(message)
92 changes: 91 additions & 1 deletion tests/utils/test_parameterized_query.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from unittest import TestCase
import pytest

from redash.utils.parameterized_query import ParameterizedQuery
from redash.utils.parameterized_query import ParameterizedQuery, InvalidParameterError


class TestParameterizedQuery(TestCase):
Expand Down Expand Up @@ -41,3 +42,92 @@ def test_handles_objects(self):
}
})
self.assertEqual(set([]), query.missing_params)

def test_raises_on_invalid_text_parameters(self):
schema = [{"name": "bar", "type": "text"}]
query = ParameterizedQuery("foo", schema)

with pytest.raises(InvalidParameterError):
query.apply({"bar": 7})

def test_validates_text_parameters(self):
schema = [{"name": "bar", "type": "text"}]
query = ParameterizedQuery("foo {{bar}}", schema)

query.apply({"bar": u"baz"})

self.assertEquals("foo baz", query.text)

def test_raises_on_invalid_number_parameters(self):
schema = [{"name": "bar", "type": "number"}]
query = ParameterizedQuery("foo", schema)

with pytest.raises(InvalidParameterError):
query.apply({"bar": "baz"})

def test_validates_number_parameters(self):
schema = [{"name": "bar", "type": "number"}]
query = ParameterizedQuery("foo {{bar}}", schema)

query.apply({"bar": 7})

self.assertEquals("foo 7", query.text)

def test_raises_on_invalid_date_parameters(self):
schema = [{"name": "bar", "type": "date"}]
query = ParameterizedQuery("foo", schema)

with pytest.raises(InvalidParameterError):
query.apply({"bar": "baz"})

def test_validates_date_parameters(self):
schema = [{"name": "bar", "type": "date"}]
query = ParameterizedQuery("foo {{bar}}", schema)

query.apply({"bar": "2000-01-01 12:00:00"})

self.assertEquals("foo 2000-01-01 12:00:00", query.text)

def test_raises_on_invalid_enum_parameters(self):
schema = [{"name": "bar", "type": "enum", "enumOptions": ["baz", "qux"]}]
query = ParameterizedQuery("foo", schema)

with pytest.raises(InvalidParameterError):
query.apply({"bar": 7})

def test_raises_on_unlisted_enum_value_parameters(self):
schema = [{"name": "bar", "type": "enum", "enumOptions": ["baz", "qux"]}]
query = ParameterizedQuery("foo", schema)

with pytest.raises(InvalidParameterError):
query.apply({"bar": "shlomo"})

def test_validates_enum_parameters(self):
schema = [{"name": "bar", "type": "enum", "enumOptions": ["baz", "qux"]}]
query = ParameterizedQuery("foo {{bar}}", schema)

query.apply({"bar": "baz"})

self.assertEquals("foo baz", query.text)

def test_raises_on_invalid_date_range_parameters(self):
schema = [{"name": "bar", "type": "date-range"}]
query = ParameterizedQuery("foo", schema)

with pytest.raises(InvalidParameterError):
query.apply({"bar": "baz"})

def test_validates_date_range_parameters(self):
schema = [{"name": "bar", "type": "date-range"}]
query = ParameterizedQuery("foo {{bar.start}} {{bar.end}}", schema)

query.apply({"bar": {"start": "2000-01-01 12:00:00", "end": "2000-12-31 12:00:00"}})

self.assertEquals("foo 2000-01-01 12:00:00 2000-12-31 12:00:00", query.text)

def test_raises_on_unexpected_param_types(self):
schema = [{"name": "bar", "type": "burrito"}]
query = ParameterizedQuery("foo", schema)

with pytest.raises(InvalidParameterError):
query.apply({"bar": "baz"})