Restrict search to items in owned histories

This was an oversight in https://github.com/galaxyproject/galaxy/pull/7745.
This is the quick fix, but I think it's feasible to implement the
accessible logic as a query, which means we could push this down to
a Mixin, and treat it as a regular fiter (so admins could for instance
search/sort for the largest datasets, which is already possible in the
reports app).
This commit is contained in:
mvdbeek
2019-04-20 16:27:34 +02:00
parent eaff93c6b5
commit 6b3aecdcc4
3 changed files with 32 additions and 6 deletions
+23 -5
View File
@@ -267,7 +267,14 @@ class HistoryContentsManager(containers.ContainerManagerMixin):
return False
return True
def _union_of_contents_query(self, container, filters=None, limit=None, offset=None, order_by=None, **kwargs):
def _union_of_contents_query(self,
container,
filters=None,
limit=None,
offset=None,
order_by=None,
user_id=None,
**kwargs):
"""
Returns a query for a limited and offset list of both types of contents,
filtered and in some order.
@@ -285,8 +292,10 @@ class HistoryContentsManager(containers.ContainerManagerMixin):
# note: I'm trying to keep these private functions as generic as possible in order to move them toward base later
# query 1: create a union of common columns for which the component_classes can be filtered/limited
contained_query = self._contents_common_query_for_contained(container.id if container else None)
subcontainer_query = self._contents_common_query_for_subcontainer(container.id if container else None)
contained_query = self._contents_common_query_for_contained(history_id=container.id if container else None,
user_id=user_id)
subcontainer_query = self._contents_common_query_for_subcontainer(history_id=container.id if container else None,
user_id=user_id)
filters = filters or []
# Apply filters that are specific to a model
@@ -322,7 +331,7 @@ class HistoryContentsManager(containers.ContainerManagerMixin):
columns.append(column)
return columns
def _contents_common_query_for_contained(self, history_id=None):
def _contents_common_query_for_contained(self, history_id, user_id):
component_class = self.contained_class
# TODO: and now a join with Dataset - this is getting sad
columns = self._contents_common_columns(component_class,
@@ -336,9 +345,15 @@ class HistoryContentsManager(containers.ContainerManagerMixin):
subquery = subquery.join(model.Dataset, model.Dataset.id == component_class.dataset_id)
if history_id:
subquery = subquery.filter(component_class.history_id == history_id)
else:
# Make sure we only return items that are user-accessible by checking that they are in a history
# owned by the current user.
# TODO: move into filter mixin, and implement accessible logic as SQL query
subquery = subquery.filter(component_class.history_id == model.History.table.c.id,
model.History.table.c.user_id == user_id)
return subquery
def _contents_common_query_for_subcontainer(self, history_id):
def _contents_common_query_for_subcontainer(self, history_id, user_id):
component_class = self.subcontainer_class
columns = self._contents_common_columns(component_class,
history_content_type=literal('dataset_collection'),
@@ -358,6 +373,9 @@ class HistoryContentsManager(containers.ContainerManagerMixin):
model.DatasetCollection.id == component_class.collection_id)
if history_id:
subquery = subquery.filter(component_class.history_id == history_id)
else:
subquery = subquery.filter(component_class.history_id == model.History.table.c.id,
model.History.table.c.user_id == user_id)
return subquery
def _get_union_type(self, union):
+1 -1
View File
@@ -115,7 +115,7 @@ class DatasetsController(BaseAPIController, UsesVisualizationMixin):
if history_id:
container = self.history_manager.get_accessible(self.decode_id(history_id), trans.user)
contents = self.history_contents_manager.contents(
container=container, filters=filters, limit=limit, offset=offset, order_by=order_by
container=container, filters=filters, limit=limit, offset=offset, order_by=order_by, user_id=trans.user.id,
)
return [self.serializer_by_type[content.history_content_type].serialize_to_view(content, user=trans.user, trans=trans, view='summary') for content in contents]
+8
View File
@@ -88,6 +88,14 @@ class DatasetsApiTestCase(api.ApiTestCase):
self._assert_status_code_is(index_response, 400)
assert index_response.json()['err_msg'] == 'bad op in filter'
def test_search_returns_only_accessible(self):
hda_id = self.dataset_populator.new_dataset(self.history_id)['id']
with self._different_user():
payload = {'limit': 10, 'offset': 0, 'q': ['history_content_type'], 'qv': ['dataset']}
index_response = self._get("datasets", payload).json()
for item in index_response:
assert hda_id != item['id']
def test_show(self):
hda1 = self.dataset_populator.new_dataset(self.history_id)
show_response = self._get("datasets/%s" % (hda1["id"]))