Skip to content

Commit

Permalink
[#1725] Adds IDataStore interface and extension point to change WHERE…
Browse files Browse the repository at this point in the history
… clauses

I prefered to extend on `where` instead of `search_data` and `delete_data`
separately to keep the extension code DRY.

The IDataStore extensions' `where` methods are called with the `filters` dict
received from the user. They must check if there's any filter they want to
process and create a `clause` list with elements as tuples following the
pattern:

    (SQL_CLAUSE, PARAM1, PARAM2, ...)

The SQL_CLAUSE can be anything valid on the WHERE clause, for example
`AVG(score) > %s AND AVG(score) < %s`. You have to substitute any user-defined
parameters for `%s`, doesn't matter what their type is (int, string, ...).
Then, for each `%s` inside SQL_CLAUSE, you need to add one PARAM. For example:

    ('AVG(score) > %s AND AVG(score) < %s', 10, 20)

This pattern is needed to be able to sanitize user input and avoid SQL
Injections.

After that, you'll need to delete every key from the `filters` dict that you've
processed. Following our example, you'll have to:

    del filters['score_between']

We need this because we validate the filters' column names against the
DataStore resource's column names. Any filter on a column that doesn't exist on
the resource is invalid and raises an error. If you didn't remove
`filters['score_between']`, you'll receive a "Field 'score_between' doesn't
exist" error.

This is done to give the user a friendlier error message, and avoid another
possible SQL Injection vector.
  • Loading branch information
vitorbaptista committed May 22, 2014
1 parent 1a14006 commit f3aa387
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 0 deletions.
7 changes: 7 additions & 0 deletions ckanext/datastore/db.py
Expand Up @@ -17,6 +17,7 @@
import psycopg2.extras
import ckan.lib.cli as cli
import ckan.plugins.toolkit as toolkit
import ckanext.datastore.interfaces as interfaces

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -757,6 +758,12 @@ def _where(field_ids, data_dict):
where_clauses = []
values = []

for plugin in p.PluginImplementations(interfaces.IDataStore):
filters, clauses = plugin.where(filters)
for clause in clauses:
where_clauses.append(clause[0])
values += clause[1:]

for field, value in filters.iteritems():
if field not in field_ids:
raise ValidationError({
Expand Down
17 changes: 17 additions & 0 deletions ckanext/datastore/interfaces.py
@@ -0,0 +1,17 @@
import ckan.plugins.interfaces as interfaces


class IDataStore(interfaces.Interface):
'''Allow changing DataStore queries'''

def where(self, filters):
'''
:param filters: dictionary with non-processed filters
:type filters: dictionary
:returns: the filters dictionary with the elements that were processed
by this method removed, and the relative clauses list created
from them.
'''
clauses = []
return filters, clauses
71 changes: 71 additions & 0 deletions ckanext/datastore/tests/test_interface.py
@@ -0,0 +1,71 @@
import nose

import ckan.plugins as p
import ckan.new_tests.helpers as helpers
import ckan.new_tests.factories as factories

import ckanext.datastore.interfaces as interfaces

assert_equals = nose.tools.assert_equals


class SampleDataStorePlugin(p.SingletonPlugin):
p.implements(interfaces.IDataStore)

def where(self, filters):
clauses = []
if 'age_between' in filters:
age_between = filters['age_between']
del filters['age_between']

clause = ('"age" >= %s AND "age" <= %s',
age_between[0], age_between[1])
clauses.append(clause)
return filters, clauses


class TestInterfaces(object):
@classmethod
def setup_class(cls):
cls.plugin = SampleDataStorePlugin()
p.load('datastore')

@classmethod
def teardown_class(cls):
p.unload('datastore')

def setup(self):
self.plugin.activate()
helpers.reset_db()

def teardown(self):
self.plugin.deactivate()

def test_can_create_custom_filters(self):
records = [
{'age': 20}, {'age': 27}, {'age': 33}
]
resource = self._create_datastore_resource(records)
filters = {'age_between': [25, 30]}

result = helpers.call_action('datastore_search',
resource_id=resource['id'],
filters=filters)

assert result['total'] == 1, result
assert result['records'][0]['age'] == 27, result
assert_equals(result['filters'], filters)

def _create_datastore_resource(self, records):
dataset = factories.Dataset()
resource = factories.Resource(package=dataset)

data = {
'resource_id': resource['id'],
'force': True,
'records': records
}

helpers.call_action('datastore_create', **data)

return resource

0 comments on commit f3aa387

Please sign in to comment.