Merge pull request #6635 from jmchilton/permission_opts

Optimize tool execution permission checking

lib/galaxy/dataset_collections/matching.py

@@ -45,6 +45,7 @@ class MatchingCollections(object):
         self.unlinked_structures = []
         self.collections = {}
         self.subcollection_types = {}
+        self.action_tuples = {}
 
     def __attempt_add_to_linked_match(self, input_name, hdca, collection_type_description, subcollection_type):
         structure = get_structure(hdca, collection_type_description, leaf_subcollection_type=subcollection_type)
@@ -76,6 +77,15 @@ class MatchingCollections(object):
         effective_structure = effective_structure.multiply(linked_structure)
         return None if effective_structure.is_leaf else effective_structure
 
+    def map_over_action_tuples(self, input_name):
+        if input_name not in self.action_tuples:
+            collection_instance = self.collections[input_name]
+            self.action_tuples[input_name] = collection_instance.collection.dataset_action_tuples
+        return self.action_tuples[input_name]
+
+    def is_mapped_over(self, input_name):
+        return input_name in self.collections
+
     @staticmethod
     def for_collections(collections_to_match, collection_type_descriptions):
         if not collections_to_match.has_collections():

lib/galaxy/model/__init__.py

@@ -3369,6 +3369,44 @@ class DatasetCollection(Dictifiable, UsesAnnotations):
             return all(e.child_collection.populated for e in self.elements)
         return top_level_populated
 
+    @property
+    def dataset_action_tuples(self):
+        if not hasattr(self, '_dataset_action_tuples'):
+            db_session = object_session(self)
+
+            dc = alias(DatasetCollection.table)
+            de = alias(DatasetCollectionElement.table)
+            hda = alias(HistoryDatasetAssociation.table)
+            dataset = alias(Dataset.table)
+            dataset_permission = alias(DatasetPermissions.table)
+
+            select_from = dc.outerjoin(de, de.c.dataset_collection_id == dc.c.id)
+
+            depth_collection_type = self.collection_type
+            while ":" in depth_collection_type:
+                child_collection = alias(DatasetCollection.table)
+                child_collection_element = alias(DatasetCollectionElement.table)
+                select_from = select_from.outerjoin(child_collection, child_collection.c.id == de.c.child_collection_id)
+                select_from = select_from.outerjoin(child_collection_element, child_collection_element.c.dataset_collection_id == child_collection.c.id)
+
+                de = child_collection_element
+                depth_collection_type = depth_collection_type.split(":", 1)[1]
+
+            select_from = select_from.outerjoin(hda, hda.c.id == de.c.hda_id).outerjoin(dataset, hda.c.dataset_id == dataset.c.id)
+            select_from = select_from.outerjoin(dataset_permission, dataset.c.id == dataset_permission.c.dataset_id)
+
+            select_stmt = select([dataset_permission.c.action, dataset_permission.c.role_id]).select_from(select_from).where(dc.c.id == self.id).distinct()
+
+            _dataset_action_tuples = []
+            for _dataset_action_tuple in db_session.execute(select_stmt).fetchall():
+                if _dataset_action_tuple[0] is None:
+                    continue
+                _dataset_action_tuples.append(_dataset_action_tuple)
+
+            self._dataset_action_tuples = _dataset_action_tuples
+
+        return self._dataset_action_tuples
+
     @property
     def waiting_for_elements(self):
         top_level_waiting = self.populated_state == DatasetCollection.populated_states.NEW

lib/galaxy/security/__init__.py

@@ -9,6 +9,7 @@ from datetime import datetime, timedelta
 from sqlalchemy import and_, false, not_, or_
 from sqlalchemy.orm import eagerload_all
 
+import galaxy.model
 from galaxy.util import listify
 from galaxy.util.bunch import Bunch
 
@@ -607,6 +608,17 @@ class GalaxyRBACAgent(RBACAgent):
         retval = self.dataset_is_public(dataset) or self.allow_action(user_roles, self.permitted_actions.DATASET_ACCESS, dataset)
         return retval
 
+    def can_access_datasets(self, user_roles, action_tuples):
+        user_role_ids = [galaxy.model.cached_id(r) for r in user_roles]
+
+        # For DATASET_ACCESS, user must have ALL associated roles
+        for action, user_role_id in action_tuples:
+            if action == self.permitted_actions.DATASET_ACCESS.action:
+                if user_role_id not in user_role_ids:
+                    return False
+
+        return True
+
     def can_manage_dataset(self, roles, dataset):
         return self.allow_action(roles, self.permitted_actions.DATASET_MANAGE_PERMISSIONS, dataset)
 
@@ -717,6 +729,27 @@ class GalaxyRBACAgent(RBACAgent):
                         perms[action].extend([_ for _ in roles if _ not in perms[action]])
         return perms
 
+    def guess_derived_permissions(self, all_input_permissions):
+        """Returns a dict of { action : [ role_id, role_id, ... ] } for the output dataset based upon input dataset permissions.
+
+        all_input_permissions should be of the form {action_name: set(role_ids)}
+        """
+        perms = {}
+        for action_name, role_ids in all_input_permissions.items():
+            if not role_ids:
+                continue
+            action = self.get_action(action_name)
+            if action not in perms.keys():
+                perms[action] = list(role_ids)
+            else:
+                if action.model == 'grant':
+                    # intersect existing roles with new roles
+                    perms[action] = [_ for _ in role_ids if _ in perms[action]]
+                elif action.model == 'restrict':
+                    # join existing roles with new roles
+                    perms[action].extend([_ for _ in role_ids if _ not in perms[action]])
+        return perms
+
     def associate_components(self, **kwd):
         if 'user' in kwd:
             if 'group' in kwd:
@@ -922,7 +955,11 @@ class GalaxyRBACAgent(RBACAgent):
             if isinstance(action, Action):
                 action = action.action
             for role in roles:
-                dp = self.model.DatasetPermissions(action, dataset, role_id=role.id)
+                if hasattr(role, "id"):
+                    role_id = role.id
+                else:
+                    role_id = role
+                dp = self.model.DatasetPermissions(action, dataset, role_id=role_id)
                 self.sa_session.add(dp)
                 flush_needed = True
         if flush_needed and flush:

lib/galaxy/tools/__init__.py

@@ -1332,6 +1332,8 @@ class Tool(Dictifiable):
                 log.error(str(exception))
                 raise exceptions.MessageException('Failure executing tool (attempting to rerun invalid job).')
 
+        set_dataset_matcher_factory(request_context, self)
+
         # Fixed set of input parameters may correspond to any number of jobs.
         # Expand these out to individual parameters for given jobs (tool executions).
         expanded_incomings, collection_info = expand_meta_parameters(trans, self, incoming)
@@ -1360,13 +1362,14 @@ class Tool(Dictifiable):
                 # Update state for all inputs on the current page taking new
                 # values from `incoming`.
                 populate_state(request_context, self.inputs, expanded_incoming, params, errors)
-
                 # If the tool provides a `validate_input` hook, call it.
                 validate_input = self.get_hook('validate_input')
                 if validate_input:
                     validate_input(request_context, errors, params, self.inputs)
             all_errors.append(errors)
             all_params.append(params)
+        unset_dataset_matcher_factory(request_context)
+
         log.debug('Validated and populated state for tool request %s' % validation_timer)
         return all_params, all_errors, rerun_remap_job_id, collection_info
 
@@ -1415,7 +1418,7 @@ class Tool(Dictifiable):
                         output_collections=execution_tracker.output_collections,
                         implicit_collections=execution_tracker.implicit_collections)
 
-    def handle_single_execution(self, trans, rerun_remap_job_id, execution_slice, history, execution_cache=None, completed_job=None):
+    def handle_single_execution(self, trans, rerun_remap_job_id, execution_slice, history, execution_cache=None, completed_job=None, collection_info=None):
         """
         Return a pair with whether execution is successful as well as either
         resulting output data or an error message indicating the problem.
@@ -1429,6 +1432,7 @@ class Tool(Dictifiable):
                 execution_cache=execution_cache,
                 dataset_collection_elements=execution_slice.dataset_collection_elements,
                 completed_job=completed_job,
+                collection_info=collection_info,
             )
         except webob.exc.HTTPFound as e:
             # if it's a webob redirect exception, pass it up the stack

lib/galaxy/tools/actions/__init__.py

@@ -58,7 +58,7 @@ class ToolAction(object):
 class DefaultToolAction(object):
     """Default tool action is to run an external command"""
 
-    def _collect_input_datasets(self, tool, param_values, trans, history, current_user_roles=None):
+    def _collect_input_datasets(self, tool, param_values, trans, history, current_user_roles=None, dataset_collection_elements=None, collection_info=None):
         """
         Collect any dataset inputs from incoming. Returns a mapping from
         parameter name to Dataset instance for each tool parameter that is
@@ -67,6 +67,12 @@ class DefaultToolAction(object):
         if current_user_roles is None:
             current_user_roles = trans.get_current_user_roles()
         input_datasets = odict()
+        all_permissions = {}
+
+        def record_permission(action, role_id):
+            if action not in all_permissions:
+                all_permissions[action] = set()
+            all_permissions[action].add(role_id)
 
         def visitor(input, value, prefix, parent=None, **kwargs):
 
@@ -85,8 +91,22 @@ class DefaultToolAction(object):
                         else:
                             data = data.get_converted_dataset(trans, target_ext, target_context=parent, history=history)
 
-                if not trans.app.security_agent.can_access_dataset(current_user_roles, data.dataset):
-                    raise Exception("User does not have permission to use a dataset (%s) provided for input." % data.id)
+                input_name = prefix + input.name
+                # Checked security of whole collection all at once if mapping over this input, else
+                # fetch dataset details for this input from the database.
+                if collection_info and collection_info.is_mapped_over(input_name):
+                    action_tuples = collection_info.map_over_action_tuples(input_name)
+                    if not trans.app.security_agent.can_access_datasets(current_user_roles, action_tuples):
+                        raise Exception("User does not have permission to use a dataset provided for input.")
+                    for action, role_id in action_tuples:
+                        record_permission(action, role_id)
+                else:
+                    if not trans.app.security_agent.can_access_dataset(current_user_roles, data.dataset):
+                        raise Exception("User does not have permission to use a dataset (%s) provided for input." % data.id)
+                    permissions = trans.app.security_agent.get_permissions(data.dataset)
+                    for action, roles in permissions.items():
+                        for role in roles:
+                            record_permission(action.action, model.cached_id(role))
                 return data
             if isinstance(input, DataToolParameter):
                 if isinstance(value, list):
@@ -137,25 +157,28 @@ class DefaultToolAction(object):
                 if not value:
                     return
 
-                dataset_instances = []
+                collection = None
                 if hasattr(value, 'child_collection'):
                     # if we are mapping a collection over a tool, we only require the child_collection
-                    dataset_instances = value.child_collection.dataset_instances
+                    collection = value.child_collection
                 else:
                     # else the tool takes a collection as input so we need everything
-                    dataset_instances = value.collection.dataset_instances
+                    collection = value.collection
 
-                for i, v in enumerate(dataset_instances):
-                    data = v
-                    if not trans.app.security_agent.can_access_dataset(current_user_roles, data.dataset):
-                        raise Exception("User does not have permission to use a dataset (%s) provided for input." % data.id)
+                action_tuples = collection.dataset_action_tuples
+                if not trans.app.security_agent.can_access_datasets(current_user_roles, action_tuples):
+                    raise Exception("User does not have permission to use a dataset provided for input.")
+                for action, role_id in action_tuples:
+                    record_permission(action, role_id)
+
+                for i, v in enumerate(collection.dataset_instances):
                     # Skipping implicit conversion stuff for now, revisit at
                     # some point and figure out if implicitly converting a
                     # dataset collection makes senese.
-                    input_datasets[prefix + input.name + str(i + 1)] = data
+                    input_datasets[prefix + input.name + str(i + 1)] = v
 
         tool.visit_inputs(param_values, visitor)
-        return input_datasets
+        return input_datasets, all_permissions
 
     def collect_input_dataset_collections(self, tool, param_values):
         def append_to_key(the_dict, key, value):
@@ -198,7 +221,7 @@ class DefaultToolAction(object):
     def _check_access(self, tool, trans):
         assert tool.allow_user_access(trans.user), "User (%s) is not allowed to access this tool." % (trans.user)
 
-    def _collect_inputs(self, tool, trans, incoming, history, current_user_roles):
+    def _collect_inputs(self, tool, trans, incoming, history, current_user_roles, collection_info):
         """ Collect history as well as input datasets and collections. """
         app = trans.app
         # Set history.
@@ -211,7 +234,7 @@ class DefaultToolAction(object):
         # input datasets can process these normally.
         inp_dataset_collections = self.collect_input_dataset_collections(tool, incoming)
         # Collect any input datasets from the incoming parameters
-        inp_data = self._collect_input_datasets(tool, incoming, trans, history=history, current_user_roles=current_user_roles)
+        inp_data, all_permissions = self._collect_input_datasets(tool, incoming, trans, history=history, current_user_roles=current_user_roles, collection_info=collection_info)
 
         # grap tags from incoming HDAs
         preserved_tags = {}
@@ -230,10 +253,9 @@ class DefaultToolAction(object):
                 if hasattr(collection, "tags"):
                     for tag in collection.auto_propagated_tags:
                         preserved_tags[tag.value] = tag
+        return history, inp_data, inp_dataset_collections, preserved_tags, all_permissions
 
-        return history, inp_data, inp_dataset_collections, preserved_tags
-
-    def execute(self, tool, trans, incoming=None, return_job=False, set_output_hid=True, history=None, job_params=None, rerun_remap_job_id=None, execution_cache=None, dataset_collection_elements=None, completed_job=None):
+    def execute(self, tool, trans, incoming=None, return_job=False, set_output_hid=True, history=None, job_params=None, rerun_remap_job_id=None, execution_cache=None, dataset_collection_elements=None, completed_job=None, collection_info=None):
         """
         Executes a tool, creating job and tool outputs, associating them, and
         submitting the job to the job queue. If history is not specified, use
@@ -245,8 +267,7 @@ class DefaultToolAction(object):
         if execution_cache is None:
             execution_cache = ToolExecutionCache(trans)
         current_user_roles = execution_cache.current_user_roles
-        history, inp_data, inp_dataset_collections, preserved_tags = self._collect_inputs(tool, trans, incoming, history, current_user_roles)
-
+        history, inp_data, inp_dataset_collections, preserved_tags, all_permissions = self._collect_inputs(tool, trans, incoming, history, current_user_roles, collection_info)
         # Build name for output datasets based on tool name and input names
         on_text = self._get_on_text(inp_data)
 
@@ -286,7 +307,7 @@ class DefaultToolAction(object):
             # Determine output dataset permission/roles list
             existing_datasets = [inp for inp in inp_data.values() if inp]
             if existing_datasets:
-                output_permissions = app.security_agent.guess_derived_permissions_for_datasets(existing_datasets)
+                output_permissions = app.security_agent.guess_derived_permissions(all_permissions)
             else:
                 # No valid inputs, we will use history defaults
                 output_permissions = app.security_agent.history_get_default_permissions(history)
@@ -490,7 +511,7 @@ class DefaultToolAction(object):
         job_setup_timer = ExecutionTimer()
         # Create the job object
         job, galaxy_session = self._new_job_for_session(trans, tool, history)
-        self._record_inputs(trans, tool, job, incoming, inp_data, inp_dataset_collections, current_user_roles)
+        self._record_inputs(trans, tool, job, incoming, inp_data, inp_dataset_collections)
         self._record_outputs(job, out_data, output_collections)
         job.object_store_id = object_store_populator.object_store_id
         if job_params:
@@ -650,7 +671,7 @@ class DefaultToolAction(object):
             job.tool_version = "1.0.0"
         return job, galaxy_session
 
-    def _record_inputs(self, trans, tool, job, incoming, inp_data, inp_dataset_collections, current_user_roles):
+    def _record_inputs(self, trans, tool, job, incoming, inp_data, inp_dataset_collections):
         # FIXME: Don't need all of incoming here, just the defined parameters
         #        from the tool. We need to deal with tools that pass all post
         #        parameters to the command as a special case.
@@ -690,7 +711,7 @@ class DefaultToolAction(object):
 
         for name, value in tool.params_to_strings(incoming, trans.app).items():
             job.add_parameter(name, value)
-        self._check_input_data_access(trans, job, inp_data, current_user_roles)
+        self._record_input_datasets(trans, job, inp_data)
 
     def _record_outputs(self, job, out_data, output_collections):
         out_collections = output_collections.out_collections
@@ -703,17 +724,10 @@ class DefaultToolAction(object):
             job.add_output_dataset_collection(name, dataset_collection_instance)
             dataset_collection_instance.job = job
 
-    def _check_input_data_access(self, trans, job, inp_data, current_user_roles):
-        access_timer = ExecutionTimer()
+    def _record_input_datasets(self, trans, job, inp_data):
         for name, dataset in inp_data.items():
-            if dataset:
-                if not trans.app.security_agent.can_access_dataset(current_user_roles, dataset.dataset):
-                    raise Exception("User does not have permission to use a dataset (%s) provided for input." % dataset.id)
-                job.add_input_dataset(name, dataset=dataset)
-            else:
-                job.add_input_dataset(name, None)
-        job_str = job.log_str()
-        log.info("Verified access to datasets for %s %s" % (job_str, access_timer))
+            # TODO: figure out why can't pass dataset_id here.
+            job.add_input_dataset(name, dataset=dataset)
 
     def get_output_name(self, output, dataset, tool, on_text, trans, incoming, history, params, job_params):
         if output.label:

lib/galaxy/tools/actions/model_operations.py

@@ -12,21 +12,21 @@ log = logging.getLogger(__name__)
 
 class ModelOperationToolAction(DefaultToolAction):
 
-    def check_inputs_ready(self, tool, trans, incoming, history, execution_cache=None):
+    def check_inputs_ready(self, tool, trans, incoming, history, execution_cache=None, collection_info=None):
         if execution_cache is None:
             execution_cache = ToolExecutionCache(trans)
 
         current_user_roles = execution_cache.current_user_roles
-        history, inp_data, inp_dataset_collections, _ = self._collect_inputs(tool, trans, incoming, history, current_user_roles)
+        history, inp_data, inp_dataset_collections, _, _ = self._collect_inputs(tool, trans, incoming, history, current_user_roles, collection_info)
 
         tool.check_inputs_ready(inp_data, inp_dataset_collections)
 
-    def execute(self, tool, trans, incoming={}, set_output_hid=False, overwrite=True, history=None, job_params=None, execution_cache=None, **kwargs):
+    def execute(self, tool, trans, incoming={}, set_output_hid=False, overwrite=True, history=None, job_params=None, execution_cache=None, collection_info=None, **kwargs):
         if execution_cache is None:
             execution_cache = ToolExecutionCache(trans)
 
         current_user_roles = execution_cache.current_user_roles
-        history, inp_data, inp_dataset_collections, preserved_tags = self._collect_inputs(tool, trans, incoming, history, current_user_roles)
+        history, inp_data, inp_dataset_collections, preserved_tags, all_permissions = self._collect_inputs(tool, trans, incoming, history, current_user_roles, collection_info)
 
         # Build name for output datasets based on tool name and input names
         on_text = self._get_on_text(inp_data)
@@ -55,7 +55,7 @@ class ModelOperationToolAction(DefaultToolAction):
         #
         job, galaxy_session = self._new_job_for_session(trans, tool, history)
         self._produce_outputs(trans, tool, out_data, output_collections, incoming=incoming, history=history, tags=preserved_tags)
-        self._record_inputs(trans, tool, job, incoming, inp_data, inp_dataset_collections, current_user_roles)
+        self._record_inputs(trans, tool, job, incoming, inp_data, inp_dataset_collections)
         self._record_outputs(job, out_data, output_collections)
         job.state = job.states.OK
         trans.sa_session.add(job)

lib/galaxy/tools/execute.py

@@ -64,7 +64,7 @@ def execute(trans, tool, mapping_params, history, rerun_remap_job_id=None, colle
             # Only workflow invocation code gets to set this, ignore user supplied
             # values or rerun parameters.
             del params['__workflow_resource_params__']
-        job, result = tool.handle_single_execution(trans, rerun_remap_job_id, execution_slice, history, execution_cache, completed_job)
+        job, result = tool.handle_single_execution(trans, rerun_remap_job_id, execution_slice, history, execution_cache, completed_job, collection_info)
         if job:
             message = EXECUTION_SUCCESS_MESSAGE % (tool.id, job.id, job_timer)
             log.debug(message)
@@ -80,7 +80,9 @@ def execute(trans, tool, mapping_params, history, rerun_remap_job_id=None, colle
                 tool,
                 trans,
                 params,
-                history
+                history,
+                execution_cache=execution_cache,
+                collection_info=collection_info,
             )
 
     execution_tracker.ensure_implicit_collections_populated(history, mapping_params.param_template)